Skip to content

[AArch64] When printing SYS aliases, use explicit NeedsReg flag from tablegen (NFC) #140484

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 19, 2025

Conversation

jthackray
Copy link
Contributor

Currently, when printing SYS aliases, the first instruction operand is compared with the string constant "all" to decide if a register needs to be parsed as the next operand.

For example, TLBI VMALLE1IS contains "all" so no register is expected, but TLBI IPAS2E1IS doesn't match, so a register is expected.

Future AArch64 SYS aliases won't always match this pattern, so use the (already provided) explicit NeedsReg bit flag provided in tablegen to check if a register is required to be parsed. This is already used by the code in AArch64InstPrinter.cpp, so now we are consistent in this source file too.

No test files have been changed, since this is a non-functional change, and all AArch64 test cases continue to pass after this change.

…m tablegen (NFC)

Currently, when printing SYS aliases, the first instruction operand
is compared with the string constant "all" to decide if a register
needs to be parsed as the next operand.

For example, `TLBI VMALLE1IS` contains "all" so no register is expected,
but `TLBI IPAS2E1IS` doesn't match, so a register is expected.

Future AArch64 SYS aliases won't match this pattern, so use the (already
provided) explicit `NeedsReg` bit flag provided in tablegen to check if a
register is required to be parsed. This is already used by the code in
`AArch64InstPrinter.cpp`, so now we are consistent in this source file too.

No test files have been changed, since this is a non-functional change,
and all AArch64 test cases continue to pass after this change.
@llvmbot
Copy link
Member

llvmbot commented May 18, 2025

@llvm/pr-subscribers-backend-aarch64

Author: Jonathan Thackray (jthackray)

Changes

Currently, when printing SYS aliases, the first instruction operand is compared with the string constant "all" to decide if a register needs to be parsed as the next operand.

For example, TLBI VMALLE1IS contains "all" so no register is expected, but TLBI IPAS2E1IS doesn't match, so a register is expected.

Future AArch64 SYS aliases won't always match this pattern, so use the (already provided) explicit NeedsReg bit flag provided in tablegen to check if a register is required to be parsed. This is already used by the code in AArch64InstPrinter.cpp, so now we are consistent in this source file too.

No test files have been changed, since this is a non-functional change, and all AArch64 test cases continue to pass after this change.


Full diff: https://github.com/llvm/llvm-project/pull/140484.diff

1 Files Affected:

  • (modified) llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp (+3-1)
diff --git a/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp b/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
index 00e8140807735..e0bd2e55cd959 100644
--- a/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
+++ b/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
@@ -3917,6 +3917,7 @@ bool AArch64AsmParser::parseSysAlias(StringRef Name, SMLoc NameLoc,
   const AsmToken &Tok = getTok();
   StringRef Op = Tok.getString();
   SMLoc S = Tok.getLoc();
+  bool ExpectRegister = true;
 
   if (Mnemonic == "ic") {
     const AArch64IC::IC *IC = AArch64IC::lookupICByName(Op);
@@ -3927,6 +3928,7 @@ bool AArch64AsmParser::parseSysAlias(StringRef Name, SMLoc NameLoc,
       setRequiredFeatureString(IC->getRequiredFeatures(), Str);
       return TokError(Str);
     }
+    ExpectRegister = IC->NeedsReg;
     createSysAlias(IC->Encoding, Operands, S);
   } else if (Mnemonic == "dc") {
     const AArch64DC::DC *DC = AArch64DC::lookupDCByName(Op);
@@ -3957,6 +3959,7 @@ bool AArch64AsmParser::parseSysAlias(StringRef Name, SMLoc NameLoc,
       setRequiredFeatureString(TLBI->getRequiredFeatures(), Str);
       return TokError(Str);
     }
+    ExpectRegister = TLBI->NeedsReg;
     createSysAlias(TLBI->Encoding, Operands, S);
   } else if (Mnemonic == "cfp" || Mnemonic == "dvp" || Mnemonic == "cpp" || Mnemonic == "cosp") {
 
@@ -3987,7 +3990,6 @@ bool AArch64AsmParser::parseSysAlias(StringRef Name, SMLoc NameLoc,
 
   Lex(); // Eat operand.
 
-  bool ExpectRegister = !Op.contains_insensitive("all");
   bool HasRegister = false;
 
   // Check for the optional register operand.

Copy link
Contributor

@Stylie777 Stylie777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jthackray jthackray requested a review from pratlucas May 19, 2025 15:04
@jthackray jthackray merged commit e8a2ce1 into main May 19, 2025
13 checks passed
@jthackray jthackray deleted the users/jthackray/use-needsreg branch May 19, 2025 16:15
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Jun 3, 2025
…m tablegen (NFC) (llvm#140484)

Currently, when printing SYS aliases, the first instruction operand is
compared with the string constant "all" to decide if a register needs to
be parsed as the next operand.

For example, `TLBI VMALLE1IS` contains "all" so no register is expected,
but `TLBI IPAS2E1IS` doesn't match, so a register is expected.

Future AArch64 SYS aliases won't always match this pattern, so use the
(already provided) explicit `NeedsReg` bit flag provided in tablegen to
check if a register is required to be parsed. This is already used by
the code in `AArch64InstPrinter.cpp`, so now we are consistent in this
source file too.

No test files have been changed, since this is a non-functional change,
and all AArch64 test cases continue to pass after this change.
ajaden-codes pushed a commit to Jaddyen/llvm-project that referenced this pull request Jun 6, 2025
…m tablegen (NFC) (llvm#140484)

Currently, when printing SYS aliases, the first instruction operand is
compared with the string constant "all" to decide if a register needs to
be parsed as the next operand.

For example, `TLBI VMALLE1IS` contains "all" so no register is expected,
but `TLBI IPAS2E1IS` doesn't match, so a register is expected.

Future AArch64 SYS aliases won't always match this pattern, so use the
(already provided) explicit `NeedsReg` bit flag provided in tablegen to
check if a register is required to be parsed. This is already used by
the code in `AArch64InstPrinter.cpp`, so now we are consistent in this
source file too.

No test files have been changed, since this is a non-functional change,
and all AArch64 test cases continue to pass after this change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants